-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Reworked background helpers #100
Conversation
…background helpers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some notes for the reviewer.
// META keep SonarCloud happy | ||
/// Init method | ||
/// - Parameter qos: Optionally set a custom QoS used by underlying queues | ||
public init(qos: DispatchQoS = .default) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can specify QoS on FlowToAsyncResult
to be consistent with other core types.
public typealias TaskCompletion = () -> Void | ||
|
||
/// Something that can perform arbitrary short background tasks, with closure API. | ||
protocol ClosureExpiringActivable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed BackgroundExecutor
into ClosureExpiringActivity
with a linked protocol ClosureExpiringActivable
to move to something testable.
|
||
/// Init method of `BackgroundExecutor` | ||
/// - Parameter qos: QoS used by the underlying queues. Defaults to `.userInitiated` to prevent most priority inversions. | ||
public init(qos: DispatchQoS = .userInitiated) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QoS of ClosureExpiringActivity
can be specified at init.
func endAll() | ||
} | ||
|
||
public final class ExpiringActivity: ExpiringActivityable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something in Drive Core
that I brought here, also cleaning it a bit, adding some tests, documenting the protocol. The whole 9 yards.
bc80e02
to
5ded5cd
Compare
8b9492f
to
2f1d979
Compare
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
#if os(macOS) | ||
// We block a non cooperative queue that matches current QoS | ||
DispatchQueue.global(qos: qos.qosClass).async { | ||
self.processInfo.performActivity(options: .suddenTerminationDisabled, reason: self.id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call in synchronous, therefore I jump queue, unlike iOS.
|
||
#if os(macOS) | ||
DDLogDebug("Starting task \(taskName) (No expiration handler as we are running on macOS)") | ||
processInfo.performActivity(options: .suddenTerminationDisabled, reason: taskName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This call been synchronous, I removed the DispatchGroup
on macOS , it works out of the box.
/// - Parameters: | ||
/// - block: The work to be performed on the background | ||
/// - onExpired: The closure called by the system when we should end. | ||
func executeWithBackgroundTask(_ block: @escaping (@escaping TaskCompletion) -> Void, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Maybe change executeWithBackgroundTask
for executeWithShortBackground
?
A form of it was introduced in Transactionable PR to harden the write DB operations. Closing as most of the work was merged on top of APIV3 with Transactionable. |
Abstract
Rationalized the various objects available in
Drive
andMail
making use ofProcessInfo.performExpiringActivity(withReason)
. Added a high level of polish, added documentation, tests, reworked the API so on so forth.NOTE: This is API breaking. Will be published in a 7.0.0
TODO: copy changes made here (bool should terminate) Infomaniak/ios-kDrive#1111